refactor: dedupe inner-tool model routing into a single helper#10
Merged
adamlu123 merged 1 commit intoMay 27, 2026
Merged
Conversation
Following up on microsoft#5: consolidate the model-config resolution helpers that were duplicated verbatim between image_qa.py and self_reflection.py into a shared `tools/_model_config.py` and drop the legacy OpenAI direct-HTTP code paths that the new model-routed path made redundant. - New `tools/_model_config.py` exposes `load_tool_model` plus `resolve_model_config_path` + `_extract_model_block`. Path resolution is now two-step: an explicit `--model-config` arg, then `<workspace_dir>/config_snapshot/merged_config.yaml` (already written by `cli.py`). - Drop the `WEBWRIGHT_TOOL_MODEL_CONFIG` env var and the `tool_model_config.json` fallback (neither was produced by anything in the repo). - Drop the `--api-key` / `--endpoint` / `--model` / `--max-attempts` / `--retry-base-delay` CLI flags from both tools; the model client owns HTTP retries via `models/base.py:_post_with_retries`. - `model_claude.yaml` no longer needs the `&model` anchor or the `tools.image_qa.model` / `tools.self_reflection.model` block; tools read the top-level `model:` block directly. Tests updated to cover the new resolver (explicit arg, snapshot fallback, missing-config error) plus a sanity check that `model_claude.yaml` has no `tools:` block. Net: -583 / +106 lines. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #5. Same behavior (Anthropic-only runs work), but smaller surface.
What changed
tools/_model_config.pyconsolidates the model-config helpers thatPR fix: route image_qa and self_reflection through the configured model (closes #3) #5 added verbatim to both
image_qa.pyandself_reflection.py.Exposes
load_tool_model,resolve_model_config_path, and_extract_model_block.--model-configarg, then<workspace_dir>/config_snapshot/merged_config.yaml(already writtenby
cli.py). Dropped theWEBWRIGHT_TOOL_MODEL_CONFIGenv var and thetool_model_config.jsonfallback — neither was produced by anythingin the repo.
(
_call_openai,_openai_config,_post_with_retry,_sleep_backoff,_RETRYABLE_STATUS_CODES) and the CLI flags they relied on(
--api-key,--endpoint,--model,--max-attempts,--retry-base-delay). The model client owns HTTP retries viamodels/base.py:_post_with_retries.model_claude.yamlno longer declares the&modelYAML anchor or thetools.image_qa.model/tools.self_reflection.modelblock — thetools read the top-level
model:block directly.Tests
tests/unit/test_tool_model_routing.pynow covers:model_claude.yamlsets a top-level Anthropic modelmodel_claude.yamlhas notools:block_extract_model_blockreads the top-level model / raises if missingresolve_model_config_pathhonors an explicit arg, falls back to thesnapshot, and raises
FileNotFoundErrorwhen neither existsAll 7 pass.
Behavior note
The
--api-key/--endpoint/--modeldirect CLI invocation paths aregone. Callers must either pass
--model-config <path>or run via theagent (which writes
config_snapshot/merged_config.yaml).Net: -583 / +106 lines vs. the state after #5 merged.